Support real-time gets on hollow shards #122012

arteam · 2025-02-07T10:53:26Z

Support returning getLastUnsafeSegmentGenerationForGets for any Engine, not only InternalEngine
Retry real-time gets on AlreadyClosedException in case a shard's engine gets swapped.

See ES-10571

…slogAction extendable

…ests" This reverts commit db73b39.

…MultiGetFomTranslogAction

server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java

server/src/main/java/org/elasticsearch/action/get/MultiGetShardRequest.java

See ES-10571

…ionForGets`

server/src/main/java/org/elasticsearch/action/get/TransportGetFromTranslogAction.java

server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java

server/src/main/java/org/elasticsearch/action/get/MultiGetShardResponse.java

server/src/main/java/org/elasticsearch/action/get/MultiGetShardRequest.java

…tFomTranslogAction extendable" This reverts commit 3237dfa

…d didn't get a cluster update

elasticsearchmachine · 2025-02-11T08:09:19Z

Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing)

fcofdez

Looks good, I left a couple of comments

server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java

fcofdez · 2025-02-11T09:21:52Z

server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java

@@ -154,7 +155,9 @@ protected Executor getExecutor(MultiGetShardRequest request, ShardId shardId) {
        final ClusterState clusterState = clusterService.state();
        if (clusterState.metadata().index(shardId.getIndex()).isSystem()) {
            return threadPool.executor(executorSelector.executorForGet(shardId.getIndexName()));
-        } else if (indicesService.indexServiceSafe(shardId.getIndex()).getIndexSettings().isSearchThrottled()) {
+        }
+        var indexService = indicesService.indexService(shardId.getIndex());


why is this change needed?

https://github.com/elastic/elasticsearch/pull/122012/files#r1950399319

I've seen an IndexNotFoundException being thrown here during the stress testing of real-time gets which failed the request, because we don't expect it here. Let me double check if it's still the case.

It used to be failing with something like

Caused by: java.lang.IllegalStateException: unexpected exception processing cluster state version [112] in context [ClusterStateObserver[null]] at org.elasticsearch.cluster.ClusterStateObserver$ObserverClusterStateListener.logUnexpectedException(ClusterStateObserver.java:317) ... 12 more Caused by: [rjsadmjle/ScBnoey6QYqfReJ_JhmkhA] org.elasticsearch.index.IndexNotFoundException: no such index [rjsadmjle] at org.elasticsearch.indices.IndicesService.indexServiceSafe(IndicesService.java:586) at org.elasticsearch.action.get.TransportGetAction.getExecutor(TransportGetAction.java:169) at org.elasticsearch.action.get.TransportGetAction.tryGetFromTranslog(TransportGetAction.java:297) at org.elasticsearch.action.get.TransportGetAction.getFromTranslog(TransportGetAction.java:263) at org.elasticsearch.action.get.TransportGetAction$1.onNewClusterState(TransportGetAction.java:241)

but it doesn't anymore, so I've reverted in in b965926

I still see the change, maybe you forgot to change this branch?

@arteam can we revert this change here (and in GET) if it's not needed? Before I approve.

@kingherc My bad, it came back with another commit. Reverted in 8d4ab8e

…t node" This reverts commit ff0c191.

kingherc

One final comment from me.

server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java

fcofdez

LGTM

…t node" This reverts commit 847bf5c.

kingherc

LGTM. Please ensure CI passes before merging.

Support returning latest hollow generation real-time GET requests

db73b39

See ES-10571

arteam added WIP :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. labels Feb 7, 2025

elasticsearchmachine added v9.1.0 serverless-linked Added by automation, don't add manually labels Feb 7, 2025

arteam added 6 commits February 8, 2025 13:39

Also support multi-get requests

0bced20

Merge remote-tracking branch 'origin/main' into hollow-real-time-gets

44fed83

Make TransportGetFromTranslogAction and TransportShardMultiGetFomTran…

3237dfa

…slogAction extendable

Revert "Support returning latest hollow generation real-time GET requ…

47b5083

…ests" This reverts commit db73b39.

Bring back original TransportGetFromTranslogAction and TransportShard…

f207d15

…MultiGetFomTranslogAction

Merge remote-tracking branch 'origin/main' into hollow-real-time-gets

75431f7

arteam changed the title ~~Support returning latest hollow generation real-time GET requests~~ Make mult-get request and response accessible outside of TransportShardMultiGetFomTranslogAction Feb 9, 2025

arteam changed the title ~~Make mult-get request and response accessible outside of TransportShardMultiGetFomTranslogAction~~ Make mult-get request and response publicly accessible Feb 9, 2025

arteam marked this pull request as ready for review February 10, 2025 08:10

Retry real-time gets for AlreadyClosedException

40301f2

arteam changed the title ~~Make mult-get request and response publicly accessible~~ Support real-time gets on hollow shards Feb 10, 2025

arteam requested review from fcofdez and kingherc February 10, 2025 11:50

arteam added 2 commits February 10, 2025 15:21

Correctly check index service in case index is not a different node

ff0c191

Merge branch 'main' into hollow-real-time-gets

634cbf4

kingherc reviewed Feb 10, 2025

View reviewed changes

server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java Show resolved Hide resolved

server/src/main/java/org/elasticsearch/action/get/MultiGetShardRequest.java Outdated Show resolved Hide resolved

arteam added 4 commits February 10, 2025 16:33

Remove access to multi get request items

e8b47d2

Support returning latest hollow generation real-time GET requests

195f5fb

See ES-10571

Throw UnsupportedOperationExceptions for `getLastUnsafeSegmentGenerat…

0ca0c78

…ionForGets`

Also support multi-get requests

87240c5

kingherc reviewed Feb 10, 2025

View reviewed changes

arteam added 4 commits February 10, 2025 22:06

Call getLastUnsafeSegmentGenerationForGets directly on Engine

25e5e9d

Revert "Make TransportGetFromTranslogAction and TransportShardMultiGe…

ed8c902

…tFomTranslogAction extendable" This reverts commit 3237dfa

Merge branch 'main' into hollow-real-time-gets

07ab4b7

Merge branch 'main' into hollow-real-time-gets

ecf8aba

Retry getTranslog calls if we encountered AlreadyClosedException an…

4806d11

…d didn't get a cluster update

arteam requested a review from kingherc February 11, 2025 07:18

arteam removed the WIP label Feb 11, 2025

Merge branch 'main' into hollow-real-time-gets

1c03540

elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label Feb 11, 2025

arteam added the >non-issue label Feb 11, 2025

fcofdez reviewed Feb 11, 2025

View reviewed changes

Revert "Correctly check index service in case index is not a differen…

b965926

…t node" This reverts commit ff0c191.

kingherc reviewed Feb 11, 2025

View reviewed changes

server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java Show resolved Hide resolved

arteam requested review from kingherc and fcofdez February 11, 2025 10:38

arteam added 3 commits February 11, 2025 11:38

Merge branch 'main' into hollow-real-time-gets

5d85f39

Correctly check index service in case index is not a different node

847bf5c

Use try versions of get*Translog calls so we don't retry forever

4bdcafb

arteam requested review from kingherc and fcofdez and removed request for fcofdez and kingherc February 11, 2025 11:58

fcofdez reviewed Feb 11, 2025

View reviewed changes

server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java Show resolved Hide resolved

server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java Show resolved Hide resolved

Add TODOs for ES-10826

01b81bb

arteam requested a review from fcofdez February 11, 2025 14:29

fcofdez approved these changes Feb 11, 2025

View reviewed changes

arteam added 2 commits February 11, 2025 16:24

Revert "Correctly check index service in case index is not a differen…

8d4ab8e

…t node" This reverts commit 847bf5c.

Merge branch 'main' into hollow-real-time-gets

d29028c

kingherc approved these changes Feb 11, 2025

View reviewed changes

arteam merged commit ecda919 into elastic:main Feb 11, 2025
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support real-time gets on hollow shards #122012

Support real-time gets on hollow shards #122012

arteam commented Feb 7, 2025 •

edited

Loading

elasticsearchmachine commented Feb 11, 2025

fcofdez left a comment

fcofdez Feb 11, 2025

arteam Feb 11, 2025

arteam Feb 11, 2025

fcofdez Feb 11, 2025

kingherc Feb 11, 2025

arteam Feb 11, 2025

kingherc left a comment

fcofdez left a comment

kingherc left a comment

Support real-time gets on hollow shards #122012

Support real-time gets on hollow shards #122012

Conversation

arteam commented Feb 7, 2025 • edited Loading

elasticsearchmachine commented Feb 11, 2025

fcofdez left a comment

Choose a reason for hiding this comment

fcofdez Feb 11, 2025

Choose a reason for hiding this comment

arteam Feb 11, 2025

Choose a reason for hiding this comment

arteam Feb 11, 2025

Choose a reason for hiding this comment

fcofdez Feb 11, 2025

Choose a reason for hiding this comment

kingherc Feb 11, 2025

Choose a reason for hiding this comment

arteam Feb 11, 2025

Choose a reason for hiding this comment

kingherc left a comment

Choose a reason for hiding this comment

fcofdez left a comment

Choose a reason for hiding this comment

kingherc left a comment

Choose a reason for hiding this comment

arteam commented Feb 7, 2025 •

edited

Loading